-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: native guest macro for kernel functions #1191
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
edsl_fp: usize, | ||
) -> Vec<MacroInstruction<F>> { | ||
match (rust_type.as_str(), edsl_type.as_str()) { | ||
("usize", "Felt < F >") => transport_felt_to_usize(rust_name, edsl_fp), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am going to develop some examples tomorrow where a const size array of Felts is converted to array of u32s. That is needed in my case (verify root proof inside a native-rv32 hybrid program).
I would suggest split this pr into multi parts: |
|
||
openvm_native_guest_macro::edsl_kernel! { | ||
fn function_name(n: usize | Felt<F>) -> usize | Felt<F> { | ||
compiler_output.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can address later, I would slightly prefer if this was a string with " " to match include_bytes!
let mut operands = vec![]; | ||
let mut j = 3; | ||
for _ in 0..num_operands { | ||
if instruction_stream[j] == VARIABLE_REGISTER_INDICATOR { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the purpose of VARIABLE_REGISTER_INDICATOR? it seems weird to have here in this general processing function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's discussed at the top of page 3 in https://docs.google.com/document/d/1_NMUff1OcJK8uf4RNgPs1xv55thyTMZ4OadqY2SFVrs/edit?usp=sharing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I somehow missed this doc :O
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can merge after addressing comments.
Will merge this version first, but noting that we already had some discussions that the "transport" layer should be moved more into something that can be done in the eDSL implementation itself, and we can layer macros on top later to help with more complicated struct transport later.
Can you add documentation around the different opcode indicators for LongForm, Gap, VariableRegister? It's hard to follow what their role is.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Commit: 53475d1 |
one thing missed i think, is the "castf" extension handling in sdk configs |
Revision of #1121. Introduces the
openvm_native_guest_macro::kernel!
macro. Its current behavior is as follows:Inside the macro, you provide a function declaration. Each type (argument types and return types) should be specified in the form
A | B
, whereA
is a Rust type andB
is an eDSL type (which will generally have a genericF
and possiblyEF
). Currently, onlyusize | Felt<F>
is supported.Inside the function body, you should provide a filename. The corresponding file should contain a series of
u32
s in plaintext format, which should be obtained from compiling an eDSL program. For each argument of the function, there should be oneu32
equal to itsfp
in the eDSL program. Then, there should be a singleu32
equal to the same for the return value. The remainingu32s
should be the serialization of the compiled program.The intention is that the eDSL program's inputs and outputs should correspond to the eDSL types specified in the macro invocation. However for now, although only
Felt<F>
is allowed by the macro, the eDSL program can use eitherFelt<F>
orUsize<F>
as their runtime behavior is the same.An example of a Fibonacci program is provided in
benchmarks/programs/test_kernel
:benchmarks/programs/test_kernel/src/main.rs
is the Rust program containing the macro invocation, andbenchmarks/programs/test_kernel/kernel/src/main.rs
is the program containing the eDSL code which is run to producebenchmarks/programs/test_kernel/compiler_output.txt
. You can runbenchmarks/src/bin/test_kernel.rs
to compile and execute this program.